Skip to content

(compat) Added supported features and generation across Loader / Driver boundary - Part 2 #24855

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Jun 17, 2025
Merged

(compat) Added supported features and generation across Loader / Driver boundary - Part 2 #24855

merged 5 commits into from
Jun 17, 2025

Conversation

agarwal-navin
Copy link
Contributor

@agarwal-navin agarwal-navin commented Jun 16, 2025 •
edited
Loading

Reviewer Guidance

Follow up to #24462 which added the logic to local driver and the validation login in the Loader layer. This PR extends the same logic to other drivers as well.

Also, note that there are a couple of test-only document service implementations to which this logic is not added - FileDocumentService and StaticStorageDocumentService. These drivers should not care about layer compatibility for now. If they do need this, it can be added later on a case-by-case basic. Keeping the logic on a need basis to keep things simple.

Description

Added logic to support layer compatibility validation at the Loader / Driver layer boundary. Note that the validation only happens in the Loader layer and not in the Driver layer. This is because Driver layer doesn't call into any objects in the Loader layer. So, it doesn't need to support compat in that direction.

For context, please look at #22877 which added the same support to the Runtime / Loader boundary. This logic added by this PR is the same as in #22877.

Supported features

  • The Driver layer adds a supportedFeatures property to the document service that includes a set of features it supports. This is advertised to the Loader layer at the layer boundary.
  • The Loader layer has a requiredFeatures array that includes a set of features that it requires the driver layer to support in order to be compatible. Note that this is internal and is not advertised.
  • At runtime, the Loader layer validates that all the entries in requiredFeatures is present in the supportedFeatures of the Driver. If not, it throws a usage error that says layers are not compatible.
  • Any change that adds new behavior or changes existing behavior across this layer boundary must add an entry to the supported features set of the Driver. For example, changes such as "does the driver support this new summary format foo" should add an entry to the supported features set of the driver.

Generation

  • In addition to supported features, a generation is also added to the Driver layer. This is advertised to the Loader layer at the layer boundary.
  • Generation is an integer which will be incremented every month. It can be used to validate compatibility more strictly (time-based) than supported features where layers are incompatible only if there are unsupported features.
    Note: The logic to update the generation will be added in a later PR. See AB#27054 for a proposed solution.
  • One key reason for adding generation is to have a clear cut off point where we officially stop supporting layer combinations. Our compat tests will test combinations up to this cut off point. The idea is that we test what we support and whatever we don't test should not be supported. This will help us achieve that.
  • The Loader layer has a minSupportedGeneration that it needs the Driver layer to be at. Note that this is internal and is not advertised.
  • At runtime, the Loader layer validates that the generation of the Driver layer is >= minSupportedGeneration.
  • For example, say that the Loader is at generation 20 and has a minSupportedGeneration of 8 for the Driver layer (12 month support window). If it sees that Driver's generation is lower than 8, the layers are incompatible.

Backwards compatibility

To support older layers before layer compat enforcement is introduced, the minSupportedGeneration is set to 0 whereas generation starts at 1. For older layers that did not implement ILayerCompatDetails, their generation is set to 0 and supported features is set to an empty set. So, their generation is compatible until we update the minSupportedGeneration.

AB#20807

@github-actions github-actions bot added area: driver Driver related issues area: loader Loader related issues area: odsp-driver area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch labels Jun 16, 2025
Copilot
Copilot AI reviewed Jun 16, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR extends loader/driver compatibility checking to all drivers by propagating supportedFeatures and generation across the Loader/Driver boundary, and updates end-to-end tests accordingly.

  • Tests: removed the local-only skip so compatibility tests run against every driver.
  • Driver layers: added ILayerCompatDetails exports and getters in DocumentService implementations.
  • Shared utilities: updated documentServiceProxy.ts to expose underlying driver compatibility details.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/test/test-end-to-end-tests/src/test/layerCompat.spec.ts Removed skip for non-local drivers in compatibility tests
packages/loader/driver-utils/src/documentServiceProxy.ts Imported and exposed ILayerCompatDetails from the underlying service
packages/drivers/routerlicious-driver/src/runtimeLayerCompatState.ts Added Routerlicious driver compat state export
packages/drivers/routerlicious-driver/src/documentService.ts Implemented ILayerCompatDetails getter in Routerlicious service
packages/drivers/replay-driver/src/replayDocumentService.ts Updated create and constructor to accept layer compat details
packages/drivers/odsp-driver/src/odspRuntimeLayerCompatState.ts Added ODSP driver compat state export
packages/drivers/odsp-driver/src/odspDocumentService.ts Implemented ILayerCompatDetails getter in ODSP service
packages/drivers/odsp-driver/src/localOdspDriver/localOdspRuntimeLayerCompatState.ts Added Local ODSP driver compat state export
packages/drivers/odsp-driver/src/localOdspDriver/localOdspDocumentService.ts Implemented ILayerCompatDetails getter in Local ODSP service
Comments suppressed due to low confidence (4)

packages/test/test-end-to-end-tests/src/test/layerCompat.spec.ts:100

  • After removing the local-only skip, tests will now run against drivers that lack ILayerCompatDetails and may fail. Add logic to skip providers without compatibility support (e.g., FileDocumentService, StaticStorageDocumentService).
provider = getTestObjectProvider();

packages/loader/driver-utils/src/documentServiceProxy.ts:31

  • [nitpick] Using the interface name ILayerCompatDetails as a property name can cause confusion. Consider renaming the property to layerCompatDetails or compatDetails.
public readonly ILayerCompatDetails: ILayerCompatDetails | undefined;

packages/drivers/routerlicious-driver/src/runtimeLayerCompatState.ts:14

  • [nitpick] The constant name r11sDriverCompatDetailsForLoader is an abbreviation (r11s) while other drivers use full names (e.g., odspDriverCompatDetailsForLoader). For consistency, consider renaming to routerliciousDriverCompatDetailsForLoader.
export const r11sDriverCompatDetailsForLoader: ILayerCompatDetails = {

packages/drivers/routerlicious-driver/src/documentService.ts:101

  • The comment refers to the 'ODSP Driver layer' in the Routerlicious driver implementation. Update this to 'Routerlicious Driver layer' to avoid confusion.
* The compatibility details of the ODSP Driver layer that is exposed to the Loader layer

@agarwal-navin agarwal-navin changed the title (compat) Added supported features and generation across Loader / Driver boundary (compat) Added supported features and generation across Loader / Driver boundary - Part 2 Jun 16, 2025
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Jun 16, 2025
@agarwal-navin agarwal-navin force-pushed the layerCompatDriverLoader branch from f716415 to 6b9c1e5 Compare June 17, 2025 00:00
scottn12
scottn12 approved these changes Jun 17, 2025
View reviewed changes
Copy link
Contributor

@scottn12 scottn12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, thanks for doing this Navin!

@agarwal-navin agarwal-navin merged commit 46c0881 into microsoft:main Jun 17, 2025
35 checks passed
@agarwal-navin agarwal-navin deleted the layerCompatDriverLoader branch June 17, 2025 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@markfields markfields markfields approved these changes

@scottn12 scottn12 scottn12 approved these changes

@vladsud vladsud Awaiting requested review from vladsud

@anthony-murphy anthony-murphy Awaiting requested review from anthony-murphy

Assignees
No one assigned
Labels
area: driver Driver related issues area: loader Loader related issues area: odsp-driver area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch dependencies Pull requests that update a dependency file
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@agarwal-navin @markfields @scottn12